Build the catalog image from a clean checkout - #5157
Draft
sir-sigurd wants to merge 5 commits into
Draft
Conversation
catalog/Dockerfile assumed pre-built assets (COPY build), with npm run build run outside Docker in the deploy workflow. docker build catalog/ therefore failed on a fresh clone, and the shipped artifact depended on the CI runner's node/npm rather than the Dockerfile. Add a node build stage and move the build context to the repo root, which the webpack build needs to resolve modules from shared/. Layer order keeps npm ci off the path of app-source edits; a Dockerfile-scoped ignore file keeps the wider context to catalog/ and shared/. The workflow drops its setup-node/npm steps and now also builds on pull requests, so Dockerfile breakage surfaces before merge instead of after. Both jobs share a buildx cache scope, replacing the setup-node npm cache the in-Docker build can no longer use. Verified: the 718 static assets are byte-identical to an out-of-Docker build of the same source at the same node/npm, and the image matches the previous one layer-for-layer in size. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #5157 +/- ##
==========================================
- Coverage 49.60% 48.23% -1.37%
==========================================
Files 844 827 -17
Lines 34472 34107 -365
Branches 5830 5830
==========================================
- Hits 17099 16451 -648
- Misses 15489 15772 +283
Partials 1884 1884
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
v3/v6 run on the deprecated Node 20 runtime; the v4/v7 majors are a runtime bump plus removal of inputs this workflow doesn't set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Reverted in the next commit. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This reverts commit 55bf4e9.
One build job now serves both events, so the PR check and the master build cannot drift apart. On master it hands the image to the push jobs as a short-lived workflow artifact — the pushed image is byte-for-byte the artifact that was built — and each registry push is an independent matrix leg, so a failed target is retried alone, replacing the implicit MP-goes-last ordering. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
catalog/Dockerfileassumed pre-built assets —COPY build— withnpm run buildrun outside Docker in the deploy workflow. Sodocker build catalog/failed on a fresh clone, and the shipped artifact depended on whatever node/npm the CI runner resolved rather than on the Dockerfile. Catalog was the only image in the repo built this way.The build now happens in a node stage inside the Dockerfile. The context moves from
catalog/to the repo root, because the webpack build resolves modules fromshared/(internals/webpack/webpack.base.jsresolve.modules, andtsconfig.jsonpaths) — app code importsschemas/*.jsonfrom there.deploy-catalog.yamldrops its setup-node/npm ci/npm run buildsteps and is restructured into a singlebuildjob plus a per-registrypushmatrix. The build job runs on pull requests too, as a build-only check: previously the image build was exercised only on master push, so Dockerfile breakage surfaced after merge, and there was no way to rehearse a change to it. Because it is one job definition, the PR check and the master build can't drift apart. Both events share one buildx GHA cache scope (master builds warm the cache PRs read; the reverse is blocked by GitHub's cache isolation), which replaces thesetup-nodenpm cache that an in-Docker build can't use.On master, the build job hands the image to the push jobs as a short-lived workflow artifact (
docker load-able tarball, 1-day retention), so each registry receives byte-for-byte the image that was built, and each push is an independent matrix job.Marketplace can't be re-pushed under the same tag, which previously forced an implicit ordering — MP pushed last — to keep the job re-runnable. The matrix dissolves that constraint: a failed target is retried alone via "Re-run failed jobs", downloading the same image artifact, without replaying pushes that already succeeded.
Verification
catalog/node_modulesand nocatalog/buildpresent.npm ci && npm run buildof the same source at the same node/npm. The image additionally contains the two pre-existingconfig.js/config.jsonsymlinks./and/config.jswithenvsubstsubstitution applied, as before.npm cistays cached and only the copy + webpack layers rebuild.Measured on this PR's own runs: 3m28s cold with no cache to read (webpack 67s, cache export 86s), and 22s when the build context is unchanged — but that second figure is the floor, not a steady state. It needs the workflow file to be the only change, since every committed file under the
catalog/**andshared/**trigger paths is also in the build context.The dominant path — app source changed, dependencies unchanged — measures 1m53s:
npm ciserved from cache, ~18s importing cached layers, 2.7s re-copying the context, webpack 68s, cache export 6.3s. Roughly a third of runs will be closer to cold, because 12 of the last 25 catalog commits also touchedcatalog/package-lock.json, which invalidates thenpm cilayer.The previous pipeline's last five master runs took 2m31s-2m51s including the three registry pushes, so it isn't directly comparable to a build-only job. The narrower comparison: both pipelines re-run webpack on any catalog change, and both cache dependency installation (
setup-nodecached the npm download, buildx now caches the wholenpm cilayer). What's genuinely added is the cache export: 86s on a cold full export, but only 6.3s on the dominant path and 1.3s when nothing changed. So on the common case the tax is single-digit seconds against a 68s webpack step that both pipelines pay.The master-only artifact hand-off (tarball export + upload in
build, download +docker loadin each push job) is new overhead that PR runs don't exercise — its real cost shows up in the first master runs after merge.Notes
catalog/CHANGELOG.mdentry: this has no runtime or user-facing effect, and every existing entry there is a product change. Happy to add one if you'd rather.catalog/Dockerfile.dockerignore(BuildKit's per-Dockerfile ignore) so no other image's build context is affected, and it is an allow-list — a new top-level directory stays out of the context by default.catalog/.dockerignoreno longer applied once the context moved, so it's removed.node:26-trixie-slimis digest-pinned, matching the lambda Dockerfiles' convention. Debian rather than alpine becauseesbuildandmsgpackr-extractship glibc prebuilds.